Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make DatabaseTasks adapters use DatabaseConfig objects #37291

Conversation

seejohnrun
Copy link
Member

We are moving to using DatabaseConfig objects everywhere inside of
Rails instead of passing around Hash objects.

This PR moves to using DatabaseConfig objects inside of the individual
database tasks adapters. In the interest of not breaking existing
adapters, we've introduced an upgrade path by which adapters can get a
hold of database configuration objects instead of Hashes by implementing
a method self.using_database_configurations?.

cc / @eileencodes

@seejohnrun seejohnrun force-pushed the use-database-config-objects-in-database-tasks branch 3 times, most recently from b7de700 to 9f49391 Compare September 25, 2019 16:32
seejohnrun added a commit to seejohnrun/rails that referenced this pull request Sep 25, 2019
We are seeing some test failures for this test in rails#37291. It looks like
what's going on is that Puma has changed the output for this command
between 4.1 and 4.2

Previously:

```
...
* Environment: development
* Listening on tcp://localhost:3000
...

```

Now:

```
...
* Environment: development
* Listening on tcp://127.0.0.1:3000
* Listening on tcp://[::1]:3000
...

```

So to get around this, instead of checking the binding address, just
check for the presence of 'Listening' generally like we do on server
start.

Co-authored-by: eileencodes <eileencodes@gmail.com>
We are moving to using `DatabaseConfig` objects everywhere inside of
Rails instead of passing around Hash objects.

This PR moves to using `DatabaseConfig` objects inside of the individual
database tasks adapters. In the interest of not breaking existing
adapters, we've introduced an upgrade path by which adapters can get a
hold of database configuration objects instead of Hashes by implementing
a method `self.using_database_configurations?`

Co-authored-by: eileencodes <eileencodes@gmail.com>
@seejohnrun seejohnrun force-pushed the use-database-config-objects-in-database-tasks branch from 9f49391 to 4ee9dab Compare September 25, 2019 17:52
carlosantoniodasilva pushed a commit that referenced this pull request Sep 25, 2019
We are seeing some test failures for this test in #37291. It looks like
what's going on is that Puma has changed the output for this command
between 4.1 and 4.2

Previously:

```
...
* Environment: development
* Listening on tcp://localhost:3000
...

```

Now:

```
...
* Environment: development
* Listening on tcp://127.0.0.1:3000
* Listening on tcp://[::1]:3000
...

```

So to get around this, instead of checking the binding address, just
check for the presence of 'Listening' generally like we do on server
start.

Co-authored-by: eileencodes <eileencodes@gmail.com>
@eileencodes eileencodes merged commit cb6db15 into rails:master Sep 25, 2019
@eileencodes eileencodes deleted the use-database-config-objects-in-database-tasks branch September 25, 2019 22:33
tagliala added a commit to tagliala/activerecord-postgis-adapter that referenced this pull request Dec 12, 2020
- ActiveRecord Autoload Change: rails/rails@b744372
- Use DatabaseConfig Instead of Hash for Tasks: rails/rails#37291
- Move ActiveRecord::Tasks::DatabaseAlreadyExists to
  ActiveRecord::DatabaseAlreadyExists: rails/rails@69700c9

Close: rgeo#322, rgeo#323
tagliala added a commit to tagliala/activerecord-postgis-adapter that referenced this pull request Dec 12, 2020
- ActiveRecord Autoload Change: rails/rails@b744372
- Use DatabaseConfig Instead of Hash for Tasks: rails/rails#37291
- Move ActiveRecord::Tasks::DatabaseAlreadyExists to
  ActiveRecord::DatabaseAlreadyExists: rails/rails@69700c9
- Wrap native adapters connection errors: rails/rails#40110

Close: rgeo#322, rgeo#323
tagliala added a commit to tagliala/activerecord-postgis-adapter that referenced this pull request Dec 12, 2020
- ActiveRecord Autoload Change: rails/rails@b744372
- Use DatabaseConfig Instead of Hash for Tasks: rails/rails#37291
- Move ActiveRecord::Tasks::DatabaseAlreadyExists to
  ActiveRecord::DatabaseAlreadyExists: rails/rails@69700c9
- Wrap native adapters connection errors: rails/rails#40110

Close: rgeo#322, rgeo#323
tagliala added a commit to tagliala/activerecord-postgis-adapter that referenced this pull request Dec 12, 2020
- ActiveRecord Autoload Change: rails/rails@b744372
- Use DatabaseConfig Instead of Hash for Tasks: rails/rails#37291
- Move ActiveRecord::Tasks::DatabaseAlreadyExists to
  ActiveRecord::DatabaseAlreadyExists: rails/rails@69700c9
- Wrap native adapters connection errors: rails/rails#40110

Close: rgeo#322, rgeo#323
tagliala added a commit to tagliala/activerecord-postgis-adapter that referenced this pull request Dec 12, 2020
- ActiveRecord Autoload Change: rails/rails@b744372
- Use DatabaseConfig Instead of Hash for Tasks: rails/rails#37291
- Move ActiveRecord::Tasks::DatabaseAlreadyExists to
  ActiveRecord::DatabaseAlreadyExists: rails/rails@69700c9
- Wrap native adapters connection errors: rails/rails#40110

Close: rgeo#322, rgeo#323
tagliala added a commit to tagliala/activerecord-postgis-adapter that referenced this pull request Dec 12, 2020
- ActiveRecord Autoload Change: rails/rails@b744372
- Use DatabaseConfig Instead of Hash for Tasks: rails/rails#37291
- Move ActiveRecord::Tasks::DatabaseAlreadyExists to
  ActiveRecord::DatabaseAlreadyExists: rails/rails@69700c9
- Wrap native adapters connection errors: rails/rails#40110

Close: rgeo#322, rgeo#323
tagliala added a commit to tagliala/activerecord-postgis-adapter that referenced this pull request Dec 12, 2020
- ActiveRecord Autoload Change: rails/rails@b744372
- Use DatabaseConfig Instead of Hash for Tasks: rails/rails#37291
- Move ActiveRecord::Tasks::DatabaseAlreadyExists to
  ActiveRecord::DatabaseAlreadyExists: rails/rails@69700c9
- Wrap native adapters connection errors: rails/rails#40110

Close: rgeo#322, rgeo#323
tagliala added a commit to tagliala/activerecord-postgis-adapter that referenced this pull request Dec 12, 2020
- ActiveRecord Autoload Change: rails/rails@b744372
- Use DatabaseConfig Instead of Hash for Tasks: rails/rails#37291
- Move ActiveRecord::Tasks::DatabaseAlreadyExists to
  ActiveRecord::DatabaseAlreadyExists: rails/rails@69700c9
- Wrap native adapters connection errors: rails/rails#40110

Close: rgeo#322, rgeo#323
tagliala added a commit to tagliala/activerecord-postgis-adapter that referenced this pull request Dec 14, 2020
- ActiveRecord Autoload Change: rails/rails@b744372
- Use DatabaseConfig Instead of Hash for Tasks: rails/rails#37291
- Move ActiveRecord::Tasks::DatabaseAlreadyExists to
  ActiveRecord::DatabaseAlreadyExists: rails/rails@69700c9
- Wrap native adapters connection errors: rails/rails#40110

Close: rgeo#322, rgeo#323
tagliala added a commit to tagliala/activerecord-postgis-adapter that referenced this pull request Dec 15, 2020
- ActiveRecord Autoload Change: rails/rails@b744372
- Use DatabaseConfig Instead of Hash for Tasks: rails/rails#37291
- Move ActiveRecord::Tasks::DatabaseAlreadyExists to
  ActiveRecord::DatabaseAlreadyExists: rails/rails@69700c9
- Wrap native adapters connection errors: rails/rails#40110

Close: rgeo#322, rgeo#323
tagliala added a commit to tagliala/activerecord-postgis-adapter that referenced this pull request Dec 17, 2020
- ActiveRecord Autoload Change: rails/rails@b744372
- Use DatabaseConfig Instead of Hash for Tasks: rails/rails#37291
- Move ActiveRecord::Tasks::DatabaseAlreadyExists to
  ActiveRecord::DatabaseAlreadyExists: rails/rails@69700c9
- Wrap native adapters connection errors: rails/rails#40110

Close: rgeo#322, rgeo#323
dazralsky pushed a commit to dazralsky/activerecord-postgis-adapter that referenced this pull request Aug 21, 2023
- ActiveRecord Autoload Change: rails/rails@b744372
- Use DatabaseConfig Instead of Hash for Tasks: rails/rails#37291
- Move ActiveRecord::Tasks::DatabaseAlreadyExists to
  ActiveRecord::DatabaseAlreadyExists: rails/rails@69700c9
- Wrap native adapters connection errors: rails/rails#40110

Close: #322, #323
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants